Skip to content

ui notifications#1622

Merged
Rainhunter13 merged 5 commits intodevfrom
agent/lam-1417/notifications-in-ui/d32ee5
Apr 10, 2026
Merged

ui notifications#1622
Rainhunter13 merged 5 commits intodevfrom
agent/lam-1417/notifications-in-ui/d32ee5

Conversation

@Rainhunter13
Copy link
Copy Markdown
Member

@Rainhunter13 Rainhunter13 commented Apr 9, 2026

Note

Medium Risk
Adds new authenticated API routes and a Postgres table to track per-user read state, plus client UI that fetches/updates this state; also introduces MQ payload-size enforcement that can now drop oversized delivery fan-outs. Risk is moderate due to new persistence and request paths affecting notification visibility and delivery reliability.

Overview
Adds in-app notifications. The project layout now includes a slide-out NotificationPanel and a sidebar bell trigger (Zustand state + SWR fetching) that renders recent report notifications and marks them read.

Introduces notification APIs + read tracking. New /api/workspaces/[workspaceId]/notifications GET/POST endpoints fetch recent ClickHouse notifications for a project and persist per-user read state in a new Postgres notification_reads table (Drizzle schema + migration), with an added isProjectInWorkspace guard.

Hardens backend fan-out. The Rust notifications handler now serializes each delivery message to pre-check size against mq_max_payload() and skips publishing/logs an error when the MQ payload limit would be exceeded.

Reviewed by Cursor Bugbot for commit db5653a. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 9, 2026

Greptile Summary

This PR introduces an in-app Notifications experience: a slide-in NotificationPanel fed by a new /api/workspaces/[workspaceId]/notifications route, persistent per-user read-state via a new notification_reads Postgres table (Drizzle migration + schema), and a NotificationTrigger bell icon in the project sidebar. On the backend, the Rust fan-out path is hardened with a mq_max_payload() size guard that skips and logs oversized NotificationDeliveryMessage payloads before they reach RabbitMQ.

  • Authorization gap (carried from prior thread): the new route uses isProjectInWorkspace but not isUserMemberOfProject, so any authenticated user who knows a valid workspaceId/projectId pair can read or mark notifications for projects they don't belong to. isUserMemberOfProject is exported from lib/authorization/index.ts and should be added to both handlers.
  • Missing error toast in handleMarkAsRead (notification-panel.tsx) violates the project's CLAUDE.md client-fetch convention — silent revert of the optimistic update gives no feedback to the user.
  • Double serialization + hot mq_max_payload() call inside the Rust fan-out loop: each delivery is serialized twice and mq_max_payload() re-reads the env var on every iteration.

Confidence Score: 4/5

Safe to merge after the authorization membership check is added to the notifications route; remaining findings are low-risk polish items.

The core feature is well-structured: parameterized queries, Zod validation, correct composite PK in the migration, SWR deduplication, and proper functional updaters in the Zustand store. The open concern is the authorization gap in the API route (flagged in a prior thread and still unaddressed), which lets any authenticated user read another project's notifications. The other findings — missing toast, double serialization, env-var hot path — are P2 style items that don't block functionality.

frontend/app/api/workspaces/[workspaceId]/notifications/route.ts — add isUserMemberOfProject check to both GET and POST handlers before serving data.

Vulnerabilities

  • Authorization bypass (frontend/app/api/workspaces/[workspaceId]/notifications/route.ts): Both GET and POST handlers validate only that the project belongs to the workspace (isProjectInWorkspace), but do not verify the calling user is a member of that workspace or project. Any authenticated user can read notification payloads or mark notifications as read for arbitrary projects by guessing a valid workspaceId/projectId pair. isUserMemberOfProject (available in lib/authorization/index.ts) should be added to both handlers.
  • No SQL injection or command injection vectors identified; ClickHouse queries use parameterized {param: Type} syntax and all inputs pass through Zod schemas.
  • No secrets or credentials introduced.

Important Files Changed

Filename Overview
app-server/src/notifications/mod.rs Adds MQ payload-size guardrail in the fan-out loop; logic is correct but double-serializes each delivery and re-evaluates mq_max_payload() per target.
frontend/app/api/workspaces/[workspaceId]/notifications/route.ts New GET/POST route for notifications; isProjectInWorkspace check only validates project membership in the workspace, not that the calling user is a member — flagged in prior review thread.
frontend/components/notifications/notification-panel.tsx New slide-in panel with SWR-backed notification list and optimistic mark-as-read; missing error toast on failed POST violates project CLAUDE.md convention.
frontend/components/notifications/notification-store.ts Minimal Zustand store with toggle/close; uses functional updater for toggle making it safe for concurrent calls.
frontend/components/notifications/notification-trigger.tsx Bell icon trigger with unread indicator; SWR key matches NotificationPanel so requests are deduplicated.
frontend/lib/actions/notifications/index.ts Server actions for fetching ClickHouse notifications and writing read-state to Postgres; uses parameterized queries and Zod validation throughout.
frontend/lib/db/migrations/0080_third_cammi.sql Creates notification_reads table with composite PK (project_id, user_id, notification_id) and cascade-delete FKs to projects and users.
frontend/lib/authorization/index.ts Adds isUserMemberOfProject, isUserMemberOfWorkspace, and getWorkspaceRole helpers; the new notification route should be using these for membership checks.
frontend/app/project/[projectId]/layout.tsx Mounts NotificationPanel inside SidebarInset which has relative and overflow-hidden, providing correct clip behavior for the slide-in animation.
frontend/components/project/sidebar/header.tsx Wires NotificationTrigger into the sidebar header, correctly hidden when sidebar is collapsed.

Reviews (4): Last reviewed commit: "fix: handle serialization failure explic..." | Re-trigger Greptile

@Rainhunter13
Copy link
Copy Markdown
Member Author

@laminar-coding-agent

validate and address open pr comments

@Rainhunter13
Copy link
Copy Markdown
Member Author

@laminar-coding-agent /autoreply start

…rthy events

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Rainhunter13
❌ greptile-apps[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@laminar-coding-agent
Copy link
Copy Markdown
Contributor

Here's a summary of the 4 PR review comments and their dispositions:

1. Missing user membership check (Cursor bot + Greptile — both P0/High)False positive. proxy.ts middleware already calls isUserMemberOfWorkspace(workspaceId, userId) for all /api/workspaces/:workspaceId/... routes before the handler runs. The isProjectInWorkspace check in the route handler adds the second layer (ensuring the project belongs to the workspace). No additional auth check needed.

2. Array index used as React list key (Greptile — P2)Valid. Fixed by replacing key={i} with key={event.trace_id} on the noteworthy events list. Each NoteworthyEvent has a trace_id field that serves as a stable identifier.

3. Zustand store race condition documentation (Greptile — P2)False positive / unnecessary. The store already uses the functional updater pattern (set((state) => ...)) which is safe, and exposes both toggle and close. Adding a comment explaining this would be over-engineering for a 4-line store.

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9bc957a. Configure here.

Instead of silently falling through with size 0 when serialization
fails, log the actual serialization error and skip the delivery.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Rainhunter13 Rainhunter13 merged commit 5a0cc2d into dev Apr 10, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants